-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FE] 지출 내역 생성하기 기능 추가 #587
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생했어요~! 코멘트를 다는 도중 잘 개선해줘서 바로 approve 달게요! 빠이팅
const {theme} = useTheme(); | ||
const amountKeypads = ['1', '2', '3', '4', '5', '6', '7', '8', '9', '00', '0', '<-']; | ||
const numberKeypads = ['1', '2', '3', '4', '5', '6', '7', '8', '9', '', '0', '<-']; | ||
|
||
const {onClickKeypad, onClickDelete, onClickDeleteAll, onClickAddAmount} = useNumberKeyboard({ | ||
type, | ||
initialValue, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
생각보다 금방 해결이 돼서 다행이에요~
@@ -7,5 +7,5 @@ export const ROUTER_URLS = { | |||
eventLogin: '/event/:eventId/login', | |||
eventManage: '/event/:eventId/admin', | |||
home: '/event/:eventId/home', | |||
addBill: 'event/:eventId/addBill', | |||
addBill: '/event/:eventId/add-bill', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍👍
useEffect(() => { | ||
document.body.style.overflow = 'hidden'; | ||
|
||
return () => { | ||
document.body.style.overflow = 'auto'; | ||
}; | ||
}, []); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오~~~ 정말 디테일한 구현 좋아요!
이 내용을 퍼널을 이용하는 다른 곳에서도 자동 적용이 되도록 처리를 해줘도 좋을 것 같아요.
const canAddMembers = nameInput && !errorMessage; | ||
|
||
const canSubmitMembers = billInfo.members.length !== 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
boolean 조건 상수화👍👍
client/src/hooks/useMembersStep.ts
Outdated
members: billInfo.members.map(member => | ||
member.id === -1 ? newMembers.members.find(m => m.name === member.name)?.id || member.id : member.id, | ||
), | ||
}); | ||
} else { | ||
postBill({ | ||
title: billInfo.title, | ||
price: Number(billInfo.price.replace(',', '')), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
새로운 멤버가 있는 경우 새로운 멤버를 postMembers를 통해 값을 받아온 뒤 postBill을 하고, 새로운 멤버가 없을 경우 postBill를 바로 실행하는 구조인 것 같아요.
지금의 구조는 postMembers를 보낸 후 useEffect를 이용해서 isSuccess상태가 될 때 요청을 날리는 방식인데, mutateAsync를 사용해서 postMembers를 await으로 기다린 후 postBill을 요청하면 더 보기 좋게 작성할 수 있을 것 같아요
const usePriceStep = ({setStep, setBillInfo}: Props) => { | ||
const handleNumberKeyboardChange = useCallback( | ||
(value: string) => { | ||
setBillInfo(prev => ({...prev, price: value})); | ||
}, | ||
[setBillInfo], | ||
); | ||
|
||
const handleNextStep = () => { | ||
setStep('title'); | ||
}; | ||
|
||
return {handleNumberKeyboardChange, handleNextStep}; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
step별로 훅을 분리하니 더 직관적으로 보이는 것 같아요
issue
구현 목적
유효성 검증
파일 분리
AddBillFunnel
에 구현되어 있어 역할 분리의 필요성을 느꼈습니다.구현 내용
AddBillFunnel.tsx
TitleStep.tsx
/^([ㄱ-ㅎ가-힣a-zA-Z0-9ㆍᆢ]\s?)*$/,
정규식에 맞지 않으면title
을 입력할 수 없습니다.FixedButton
을 클릭하는 것과 Enter 키를 입력하는 것 모두 해당합니다.MemberStep.tsx
/^([ㄱ-ㅎ가-힣a-zA-Zㆍᆢ]\s?)*$/,
정규식에 맞지 않으면name
을 입력할 수 없습니다.memberId
가-1
이 있는지 확인합니다.-1
id가 있다면, 현재 선택된 member들 중 id가 없는 이름들을 배열로postMembers API
를 요청합니다.posetMembers API
응답으로 얻은memberId
를 통해postBill API
를 요청합니다.postBill API
를 요청합니다.TroubleShooting
TitleStep
에서 이전으로 버튼을 눌러PriceStep
으로 이동했을 때, NumberKeyboard가 초기화 되던 문제NumberKeyboard
가 렌더링되면서,useNumberKeyboard
내부의 value가''
으로 초기화되는 것이 원인NumberKeyboard
에 initialValue를 추가함으로써 이 문제를 해결before
논의하고 싶은 부분(선택)
postBill
부분의 API 설계상 새로운 멤버가 생겼다면 요청을 2번 보내야 하다 보니 로직적으로 복잡해 진 부분이 있는 것 같아요. 이부분에 대해서 어떻게 생각하시는지, 좋은 방법이 있으신지 궁금합니다~!